[Agent] Enhance `FileManager` and simplify the use of file managers #193

Bobholamovic · 2023-12-21T11:40:54Z

主要改动如下：

优化FileManager的使用方式。具体而言：对于初级用户，修改全局file manager的使用方式，提供配置全局file manager的功能；对于进阶用户，提供基础的资源管理功能，允许更精细化的资源、对象生命周期控制。以下是对于初级用户的示例用法。用户不需要关心file manager，只需要设置环境变量或全局配置一次。

import os

# 设置环境变量
os.environ["EB_AGENT_ACCESS_TOKEN"] = "xxx"

# 构建和使用各组件，不需要设置access_token，也不需要构建或传递file_manager

# 未设置环境变量

from erniebot_agent.file_io import GlobalFileManagerHandler

async def main():
    # 文件保存在指定位置，且文件夹不会在程序结束后被自动清理
    await GlobalFileManagerHandler().configure(access_token="xxx", save_dir="my/save/dir", prune_on_close=False)
    
    # 构建和使用各组件，不需要构建或传递file_manager

支持从环境变量读取全局access token等配置项。
新增File.write_contents_to方法，用于将文件内容转存到本地文件中。
修复Makefile中的bug。

更多细节详见comments。

Bobholamovic · 2023-12-21T11:41:21Z

erniebot-agent/Makefile

@@ -1,6 +1,9 @@
-.DEFAULT_GOAL = format lint type_check
+.DEFAULT_GOAL = dev


.DEFAULT_GOAL不能是多目标

Bobholamovic · 2023-12-21T11:41:51Z

erniebot-agent/src/erniebot_agent/agents/base.py

-from erniebot_agent.utils.logging import logger
+from erniebot_agent.utils.mixins import GradioMixin
+
+logger = logging.getLogger(__name__)


采用统一的logging风格，后同。

Bobholamovic · 2023-12-21T11:42:20Z

erniebot-agent/src/erniebot_agent/agents/base.py

@@ -27,15 +28,16 @@
    ToolResponse,
 )
 from erniebot_agent.chat_models.base import ChatModel
+from erniebot_agent.file_io import protocol


直接使用模块protocol，而不是从中import方法，这样有利于测试时做patch。后同。

Bobholamovic · 2023-12-21T11:43:51Z

erniebot-agent/src/erniebot_agent/agents/base.py

-                    if file is None:
-                        raise RuntimeError(f"Unregistered ID {repr(val)} is used by {repr(tool)}.")
-                elif is_remote_file_id(val):
+                if protocol.is_file_id(val):


重构后FileManager不再考虑多FileManager共享一个FileRegistry的情形，这大大简化了我们要处理的情况，例如这里只需要判断FileManager在自己的registry中持有的文件。

Bobholamovic · 2023-12-21T12:56:59Z

erniebot-agent/src/erniebot_agent/agents/callback/callback_manager.py

@@ -40,14 +40,14 @@ def handlers(self) -> List[CallbackHandler]:

    def add_handler(self, handler: CallbackHandler):
        if handler in self._handlers:
-            raise RuntimeError(f"The callback handler {handler} is already registered.")
+            raise ValueError(f"The callback handler {handler} is already registered.")


按照Python文档中的定义调整异常类型，后同。

erniebot-agent/src/erniebot_agent/file_io/file_registry.py

erniebot-agent/src/erniebot_agent/file_io/global_file_manager.py

Bobholamovic · 2023-12-21T13:15:31Z

erniebot-agent/tests/unit_tests/testing_utils/mocks/mock_remote_file_client_server.py

@@ -0,0 +1,119 @@
+import contextlib


后续可用于模拟文件服务器

Bobholamovic · 2023-12-21T13:17:17Z

erniebot-agent/tests/unit_tests/agents/test_agent_response_annotations.py

-class TestAgentResponseAnnotations(unittest.TestCase):
-    def setUp(self):
+class TestAgentResponseAnnotations(unittest.IsolatedAsyncioTestCase):
+    async def asyncSetUp(self):


资源有申请就有释放。后同。

codecov-commenter · 2023-12-21T13:20:55Z

Codecov Report

Attention: 148 lines in your changes are missing coverage. Please review.

Comparison is base (854b8e2) 65.07% compared to head (04d6ee1) 64.77%.

Files	Patch %	Lines
...erniebot_agent/file/global_file_manager_handler.py	34.78%	30 Missing ⚠️
...ebot-agent/src/erniebot_agent/file/file_manager.py	73.33%	28 Missing ⚠️
...iebot-agent/src/erniebot_agent/file/remote_file.py	35.29%	22 Missing ⚠️
erniebot-agent/src/erniebot_agent/utils/mixins.py	55.81%	19 Missing ⚠️
erniebot-agent/src/erniebot_agent/agents/base.py	50.00%	13 Missing ⚠️
...t-agent/src/erniebot_agent/tools/remote_toolkit.py	60.00%	6 Missing ⚠️
erniebot-agent/src/erniebot_agent/file/protocol.py	61.53%	5 Missing ⚠️
erniebot-agent/src/erniebot_agent/utils/logging.py	28.57%	5 Missing ⚠️
...bot-agent/src/erniebot_agent/file/file_registry.py	66.66%	4 Missing ⚠️
erniebot-agent/src/erniebot_agent/file/base.py	57.14%	3 Missing ⚠️
... and 7 more

Additional details and impacted files

@@             Coverage Diff             @@
##           develop     #193      +/-   ##
===========================================
- Coverage    65.07%   64.77%   -0.30%     
===========================================
  Files           56       57       +1     
  Lines         2789     2947     +158     
===========================================
+ Hits          1815     1909      +94     
- Misses         974     1038      +64

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

Southpika · 2023-12-22T06:25:36Z

erniebot-agent/src/erniebot_agent/file_io/global_file_manager.py

+    access_token: Optional[str], save_dir: Optional[str], **opts: Any
+) -> FileManager:
+    if access_token is None:
+        access_token = C.get_global_access_token()


是否需要将eb模型和file在环境变量中的ak进行强绑定呢～用户如果在环境变量中设置了ak的话就无法使用local file了。

不知道有没有一种情况用户只想将ak用于eb的模型调起但是他这个ak没有开通remote file的服务。

已增加enable_remote_file选项控制是否启用remote file管理功能。

Southpika · 2023-12-22T09:12:39Z

erniebot-agent/src/erniebot_agent/agents/base.py

-                else:
-                    continue
-                agent_files.append(AgentFile(file=file, type=file_type, used_by=tool.tool_name))
+                        raise RuntimeError(f"Unregistered file with ID {repr(val)} is used by {repr(tool)}.")


感觉是不是可以把file type拼进去，用户可以知道是输入的文件出问题还是输出的？

juncaipeng · 2023-12-22T09:21:06Z

erniebot-agent/src/erniebot_agent/file/global_file_manager.py

+    return _global_file_manager
+
+
+async def configure_global_file_manager(


这里需要一个参数来设置使用是否使用remote_file，就是上午说的需要一个开关。用户设置了access token，是希望使用aistudio的tool，而不希望使用remote file。

已增加enable_remote_file选项控制是否启用remote file管理功能。

juncaipeng · 2023-12-22T09:25:18Z

erniebot-agent/src/erniebot_agent/file/file_manager.py

        save_dir: Optional[FilePath] = None,
+        *,
+        default_file_type: Optional[Literal["local", "remote"]] = None,
+        prune_on_close: bool = True,


prune_on_close参数主要什么作用？

这个参数可以控制是否在对象close时自动清理那些可以安全删除的文件，如果设置为False的话文件会被保留

juncaipeng · 2023-12-22T09:27:11Z

erniebot-agent/src/erniebot_agent/file/global_file_manager.py

+    return _global_file_manager
+
+
+async def configure_global_file_manager(


改成set_global_file_manager?

这个后来想了下，原因如下：

不过刚才的PR有个asyncio的data race问题，需要引入锁，而asyncio.Lock作为模块全局变量可能导致一些问题（例如用户使用自定义的event loop时，锁可能不起作用），为此我改写了一个lazy Singleton，该类需要在异步函数中构造。

最新的代码里提供了set方法，用于直接设置FileManager对象。

Southpika · 2023-12-22T09:27:36Z

更新之后factory的get_file_manager好像没有地方使用了，这个文件可以删除吗

Bobholamovic · 2023-12-22T09:55:20Z

更新之后factory的get_file_manager好像没有地方使用了，这个文件可以删除吗

已删除

Southpika · 2023-12-22T10:00:40Z

erniebot-agent/src/erniebot_agent/agents/base.py

-                            f"A file is used by {repr(tool)}, but the agent has no file manager to fetch it."
+                        raise RuntimeError(
+                            f"Unregistered file with ID {repr(val)} is used by {repr(tool)}."
+                            f" File type: {file_type}"


突然发现在look_up_file_by_id中如果file为None已经抛出FileError了

Southpika · 2023-12-22T10:04:54Z

erniebot-agent/src/erniebot_agent/agents/base.py

-                    file = self._file_manager.look_up_file_by_id(val)
+                if protocol.is_file_id(val):
+                    file_manager = await self._get_file_manager()
+                    file = file_manager.look_up_file_by_id(val)


看是否要在这个地方try catch还是让他自然报错

已修改为try-catch

…nvirons

…addlePaddle#193) * Fix makefiles * Fix bugs * Enhance file_io * Update library code * Update examples * Update tests * Fix exceptions * Fix error info * Remove use of environment variables in integration tests * Fix protocol * Fix type hints * Fix * Fix typing * Fix style * Fix cleanup bugs * Fix data race * Fix bugs * Show file type * Fix bugs * Fix linting issues * Fix linting issues * Fix integration tests * Remove unused file * Fix and enhance * Fix style * Fix CI * Update CI config * Fix style * Remove anchors * Enhance mixins

Bobholamovic added 9 commits December 21, 2023 18:08

Fix makefiles

260e9bc

Fix bugs

8b23664

Enhance file_io

0a9e9e6

Update library code

38cb8ff

Update examples

f178300

Update tests

4cd12a1

Fix exceptions

23287ad

Fix error info

4807109

Remove use of environment variables in integration tests

bc8bbd8

Bobholamovic marked this pull request as draft December 21, 2023 11:44

Fix protocol

b280333

Bobholamovic commented Dec 21, 2023

View reviewed changes

Bobholamovic marked this pull request as ready for review December 21, 2023 13:17

Fix type hints

f4f61e6

Bobholamovic requested review from juncaipeng and sijunhe December 21, 2023 13:26

Bobholamovic marked this pull request as draft December 21, 2023 18:54

Southpika reviewed Dec 22, 2023

View reviewed changes

Bobholamovic added 4 commits December 22, 2023 16:23

Fix

0b02a1a

Fix typing

82b63a5

Merge remote-tracking branch 'official' into agent/feat/add_environs

1a5157b

Fix style

d54a93b

Bobholamovic marked this pull request as ready for review December 22, 2023 08:38

Fix cleanup bugs

7df8462

Southpika reviewed Dec 22, 2023

View reviewed changes

Fix data race

dc5024a

juncaipeng reviewed Dec 22, 2023

View reviewed changes

Fix bugs

866ba6d

Show file type

29be29b

Southpika reviewed Dec 22, 2023

View reviewed changes

Bobholamovic added 4 commits December 22, 2023 18:57

Fix bugs

b74ead7

Merge branch 'develop' into agent/feat/add_environs

961faea

Fix linting issues

9b73220

Fix linting issues

b19b42a

Bobholamovic requested review from Southpika and juncaipeng December 22, 2023 11:07

Bobholamovic added 10 commits December 22, 2023 19:26

Fix integration tests

1f28164

Remove unused file

d372395

Fix and enhance

5e3273d

Fix style

c0858c9

Fix CI

1e6c17f

Update CI config

f84514d

Merge remote-tracking branch 'official/develop' into agent/feat/add_e…

87ae9b2

…nvirons

Fix style

275c572

Remove anchors

357a28c

Enhance mixins

04d6ee1

sijunhe approved these changes Dec 25, 2023

View reviewed changes

sijunhe merged commit 8937548 into PaddlePaddle:develop Dec 25, 2023
6 of 7 checks passed

Bobholamovic deleted the agent/feat/add_environs branch December 25, 2023 09:06

Bobholamovic mentioned this pull request Dec 25, 2023

[Agent] Fix Singleton #212

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[Agent] Enhance `FileManager` and simplify the use of file managers #193

[Agent] Enhance `FileManager` and simplify the use of file managers #193

Bobholamovic commented Dec 21, 2023 •

edited

Loading

Bobholamovic Dec 21, 2023

Bobholamovic Dec 21, 2023

Bobholamovic Dec 21, 2023

Bobholamovic Dec 21, 2023

Bobholamovic Dec 21, 2023

Bobholamovic Dec 21, 2023

Bobholamovic Dec 21, 2023

codecov-commenter commented Dec 21, 2023 •

edited

Loading

Southpika Dec 22, 2023 •

edited

Loading

Bobholamovic Dec 22, 2023 •

edited

Loading

Southpika Dec 22, 2023

Bobholamovic Dec 22, 2023

juncaipeng Dec 22, 2023

Bobholamovic Dec 22, 2023 •

edited

Loading

juncaipeng Dec 22, 2023

Bobholamovic Dec 22, 2023

juncaipeng Dec 22, 2023

Bobholamovic Dec 22, 2023 •

edited

Loading

Bobholamovic Dec 24, 2023

Southpika commented Dec 22, 2023

Bobholamovic commented Dec 22, 2023

Southpika Dec 22, 2023

Bobholamovic Dec 22, 2023

Southpika Dec 22, 2023

Bobholamovic Dec 22, 2023

		@@ -1,6 +1,9 @@
		.DEFAULT_GOAL = format lint type_check
		.DEFAULT_GOAL = dev

		return _global_file_manager


		async def configure_global_file_manager(

[Agent] Enhance FileManager and simplify the use of file managers #193

[Agent] Enhance FileManager and simplify the use of file managers #193

Conversation

Bobholamovic commented Dec 21, 2023 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

codecov-commenter commented Dec 21, 2023 • edited Loading

Codecov Report

Southpika Dec 22, 2023 • edited Loading

Choose a reason for hiding this comment

Bobholamovic Dec 22, 2023 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Bobholamovic Dec 22, 2023 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Bobholamovic Dec 22, 2023 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Southpika commented Dec 22, 2023

Bobholamovic commented Dec 22, 2023

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

[Agent] Enhance `FileManager` and simplify the use of file managers #193

[Agent] Enhance `FileManager` and simplify the use of file managers #193

Bobholamovic commented Dec 21, 2023 •

edited

Loading

codecov-commenter commented Dec 21, 2023 •

edited

Loading

Southpika Dec 22, 2023 •

edited

Loading

Bobholamovic Dec 22, 2023 •

edited

Loading

Bobholamovic Dec 22, 2023 •

edited

Loading

Bobholamovic Dec 22, 2023 •

edited

Loading